Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MPDX-7878 Performance Enhancements and Codebase Simplification #950

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

dr-bizz
Copy link
Contributor

@dr-bizz dr-bizz commented Jun 6, 2024

Description

This pull request includes several significant improvements and optimizations to enhance the performance and maintainability of our application:

  1. Loading Fonts via Font-Face: Implemented font loading through the font-face method for faster and more efficient font rendering.

  2. Condensing CSS into One File: Consolidated multiple CSS files into a single file to reduce HTTP requests and improve load times.

  3. Simplifying the Codebase: Refactored and simplified the codebase for better readability and maintainability.
    Adding Skeletons to Contacts and Tasks: Introduced skeleton loaders for Contacts and Tasks to improve user experience during data loading.

  4. Removing Delay on Dashboard: Eliminated unnecessary delays on the dashboard for a smoother and more immediate user experience.

Checklist:

  • I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
  • I have applied the appropriate labels. (Add the label "On Staging" to get the branch automatically merged into staging.)
  • I have requested a review from another person on the project

Condensing css into one file
Simplifying the codebase
@dr-bizz dr-bizz requested a review from caleballdrin June 6, 2024 12:55
@dr-bizz dr-bizz added the Preview Environment Add this label to create an Amplify Preview label Jun 6, 2024
Copy link

github-actions bot commented Jun 6, 2024

Preview branch generated at https://lazyload-components.d3dytjb8adxkk5.amplifyapp.com

Copy link

github-actions bot commented Jun 6, 2024

Bundle sizes [mpdx-react]

Compared against f58d594

Route Size (gzipped) Diff
/_app 372.01 KB +1.47 KB

@dr-bizz dr-bizz mentioned this pull request Jun 6, 2024
3 tasks
@wrandall22
Copy link
Contributor

run lighthouse

@wrandall22
Copy link
Contributor

Looks like github.head_ref can be inconsistent (see this discussion). This seems to be causing the lighthouse action to fail. They recommend using github.event.pull_request.head.ref, maybe we can do something like https://${{ github.head_ref || github.event.pull_request.head.ref }} (not sure if that syntax works in GHA or not) to get around this.

@dr-bizz
Copy link
Contributor Author

dr-bizz commented Jun 7, 2024

run lighthouse

@dr-bizz dr-bizz added Preview Environment Add this label to create an Amplify Preview and removed Preview Environment Add this label to create an Amplify Preview labels Jun 7, 2024
Copy link

github-actions bot commented Jun 7, 2024

Preview branch generated at https://lazyload-components.d3dytjb8adxkk5.amplifyapp.com

@caleballdrin
Copy link
Contributor

I think the skeletons look great and definitely help with the UX. I noticed that when you stop scrolling it flashes to the skeletons. I think it was doing that before this PR but flashing to blank instead of the skeletons.

contacts.loading.flashing.mov

@dr-bizz
Copy link
Contributor Author

dr-bizz commented Jun 11, 2024

I think the skeletons look great and definitely help with the UX. I noticed that when you stop scrolling it flashes to the skeletons. I think it was doing that before this PR but flashing to blank instead of the skeletons.

contacts.loading.flashing.mov

So with this, this is actually a feature, not a bug. when you scroll up, we load the tasks above, but there is a split second where the tasks aren't loaded in, so we show the skeletons. src/components/InfiniteList/InfiniteList.tsx - line 117

ScrollSeekPlaceholder: Skeleton ? Skeleton : SkeletonItem,

@caleballdrin
Copy link
Contributor

@dr-bizz this seems like a bug to me because it loads and all the data is on the screen, then it flashes the skeletons, then it shows the loaded data again. Are you sure it isn't re-rendering an additional time unnecessarily?

{`!function(a,e,t,n,s){a=a[s]=a[s]||{q:[],onReady:function(e){a.q.push(e)}},(s=e.createElement(t)).async=1,s.src=n,(n=e.getElementsByTagName(t)[0]).parentNode.insertBefore(s,n)}(window,document,"script","https://www.datadoghq-browser-agent.com/datadog-rum-v5.js","DD_RUM"),DD_RUM.onReady(function(){DD_RUM.init({clientToken:"${process.env.DATADOG_CLIENT_TOKEN}",applicationId:"${process.env.DATADOG_APP_ID}",site:"datadoghq.com",service:"mpdx-web-react",sessionSampleRate:100,sessionReplaySampleRate:20,trackUserInteractions:!0,trackResources:!0,trackLongTasks:!0,defaultPrivacyLevel:"mask-user-input",env:"${process.env.DD_ENV}",allowedTracingUrls:["${process.env.API_URL}"]})});`}
</Script>
)}
{process.env.GOOGLE_TAG_MANAGER_CONTAINER_ID && (
<Script id="google-analytics" strategy="afterInteractive">
<Script id="google-analytics" strategy="lazyOnload">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how much we use these analytics but do you think we will miss any data if it is lazy loaded?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Preview Environment Add this label to create an Amplify Preview
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants